serial: T8375: use boot activation script to define a serial console on the CLI#5092
serial: T8375: use boot activation script to define a serial console on the CLI#5092c-po wants to merge 5 commits intovyos:currentfrom
Conversation
|
👍 |
zdc
left a comment
There was a problem hiding this comment.
Overall, it looks logical. Let's try to polish it, I think.
-
I think, if we use only one way to note the console, it will be less messy in the code:
console_type + console_numorconsole_device. -
There is a mix-up between
strandNonearguments in functions and their calls, which should work fine generally. But it may also end up with unexpected surprises in the long term, when someone will update calls. I would prefer to unify all calls to a single type. Also, we may consider using just the explicit defaultsargument: str = ''more for "cleanup" operations to keep the mistake risk surface purely inside functions.
ed11d93 to
31f0e14
Compare
d1d817b to
bd95a69
Compare
|
Smoketests are failing due to unmerged vyos/vyos-build#1152 |
bd95a69 to
28c3206
Compare
sever-sever
left a comment
There was a problem hiding this comment.
Add console parameters based on architecture
At least we see the login/password for the git ARM runner
DEBUG - Welcome to VyOS - vyos ttyAMA0
DEBUG -
DEBUG - vyos login: vyos
DEBUG - vyos
DEBUG - Password: vyos
DEBUG -
DEBUG - Welcome to VyOS!
DEBUG -
DEBUG - ┌── ┐
DEBUG - . VyOS 999.202604030943
DEBUG - └ ──┘ current
DEBUG -
DEBUG - * Documentation: https://docs.vyos.io/en/latest
DEBUG - * Project news: https://blog.vyos.io/
DEBUG - * Bug reports: https://vyos.dev/
DEBUG -
DEBUG - You can change this banner using "set system login banner post-login" command.
DEBUG -
DEBUG - VyOS is a free software distribution that includes multiple components,
DEBUG - you can check individual component licenses under /usr/share/doc/*/copyright
DEBUG -
DEBUG - ---
DEBUG - WARNING: This VyOS system is not a stable long-term support version and
DEBUG - is not intended for production use.
DEBUG -
INFO - Logged in!
DEBUG - vyos@vyos:~$ if sudo grep -rq "BEGIN PRIVATE KEY" /var/lib/shim-signed/mok; then echo Found private key - bailing out; exit 1; fi
DEBUG - if sudo grep -rq "BEGIN PRIVATE KEY" /var/lib/shim-signed/mok; then
echo Found private key - bailing out; exit 1; fi
INFO - Starting installer
DEBUG - vyos@vyos:~$ install image
DEBUG - install image
DEBUG - Welcome to VyOS installation!
DEBUG - This command will install VyOS to your permanent storage.
DEBUG - Would you like to continue?y
DEBUG - [y/N] y
DEBUG - What would you like to name this image? (Default: 999.202604030943)
DEBUG -
DEBUG - Please enter a password for the "vyos" user: vyos
DEBUG -
DEBUG -
DEBUG - WARNING: Default password used. Consider changing it on next login.
DEBUG -
DEBUG - Please confirm password for the "vyos" user: vyos
DEBUG -
DEBUG - What console should be used by default? (K: KVM, S: Serial)? (DefauS
DEBUG - lt: A) S
DEBUG - Invalid value, try again.
ERROR - Timeout waiting for VyOS system
ERROR - Traceback (most recent call last):
File "/__w/vyos-build-arm64/vyos-build-arm64/vyos-build/scripts/check-qemu-install", line 692, in <module>
c.expect('\nWhich one should be used for installation?.*')
File "/usr/lib/python3/dist-packages/pexpect/spawnbase.py", line 343, in expect
return self.expect_list(compiled_pattern_list,
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/lib/python3/dist-packages/pexpect/spawnbase.py", line 372, in expect_list
return exp.expect_loop(timeout)
^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/lib/python3/dist-packages/pexpect/expect.py", line 181, in expect_loop
return self.timeout(e)
^^^^^^^^^^^^^^^
File "/usr/lib/python3/dist-packages/pexpect/expect.py", line 144, in timeout
raise exc
pexpect.exceptions.TIMEOUT: Timeout exceeded.
8886c2e to
36ef074
Compare
36ef074 to
b8497c3
Compare
jestabro
left a comment
There was a problem hiding this comment.
I see one issue here: the setting added to the config file during live boot (say, ttyS0) remains after the first boot into the installed image. Consequently, if a user chooses K(VM) during image install, that is correctly written to the grub vars for the reboot into the installed image; however, during that reboot, the system_console config script will re-write the config entry to the grub vars. A subsequent reboot will replace the choice of tty with ttyS.
e3624b1 to
335c6b3
Compare
The issue has been corrected. Thanks for testing this interesting combination. |
jestabro
left a comment
There was a problem hiding this comment.
The issue (mentioned above) with overwriting grub vars after installation has been resolved. The necessary tools to define a serial console on live-boot are in place.
zdc
left a comment
There was a problem hiding this comment.
The logic looks mainly solid. I have only a few questions and suggestions to clarify.
python/vyos/utils/kernel.py
Outdated
| import re | ||
| from vyos.utils.file import read_file | ||
| CMDLINE_CONSOLE_RE = re.compile( | ||
| r'(?:^|\s)console=(?P<device>tty(?:S|AMA)\d+),(?P<speed>\d+)(?=\s|$)' |
There was a problem hiding this comment.
I'm not sure if this is possible, after this change, but to clarify: is speed always presented in the cmdline? Can there be totally no scenarios in which it is missing?
I'm just thinking about corner cases when, for instance, in PXE boot, users have a full control over the cmdline and can set whatever they want there.
There was a problem hiding this comment.
If our image is used console speed is always present and defaulted to 115200 baud.
|
|
||
| speed = console['device']['ttyS0']['speed'] | ||
| grub_util.update_console_speed(speed) | ||
| if not use_serial_console: |
There was a problem hiding this comment.
Why a dedicated variable and not simply else:?
There was a problem hiding this comment.
This is needed b/c of multiple serial console support, but only one is allowed to carry the kernel CLI option due to Kernel constraints.
If the code would look like:
if 'kernel' in device_config:
# get console type ("ttyS" or "ttyAMA") from device (e.g. "ttyS0")
console_type = device.rstrip('0123456789')
console_num = device[len(console_type):]
grub_util.update_serial_console(console_type, console_num, device_config['speed'])
use_serial_console = True
else:
grub_util.update_serial_console(*default_tty_console)And I would have:
set system console device ttyS0 kernel
set system console device ttyS1
The GRUB console would be set to serial first (if-branch is true), but on the second iteration for ttyS1, there is no kernel parameter (else path) and the GRUB console would be switched back to tty0 defined by default_tty_console.
| # default boot variables | ||
| DEFAULT_BOOT_VARS: dict[str, str] = { | ||
| 'timeout': '5', | ||
| 'console_type': 'tty', |
There was a problem hiding this comment.
It is not clear why there is no flavor_sercon_type here.
Yes, it is overwritten later during installation anyway, but this creates an inconsistency.
There was a problem hiding this comment.
flavor_sercon_type is never used in GRUB - its a variable mapped later to a GRUB variable.
| if not is_live_boot(): | ||
| return |
There was a problem hiding this comment.
Nothing needs to be changed here, but if we use this condition, then we must also update all release flavors with a serial console to have a non-default config.boot file with a serial console inside (check kvm as an example of such a flavor). Otherwise, they will be locked out of the console right after boot.
There was a problem hiding this comment.
KVM should fallback to tty0, but yes - we need to look into the flavor files as kvm indeed contains some "special"
# GRUB console settings
[boot_settings]
console_type = "ttyS"
console_num = '0'
console_speed = "115200"
Which by coincidence matches to the amd64 settings and should be properly consolidated
42f7c7e to
41c3b04
Compare
41c3b04 to
625e66d
Compare
For the sake of nice grep lines and refactoring we have an unspoken - unwritten rule considered as folklore to have imports one per line. This helped in the past with refactorings.
Clearly indicate during startup what happens. When we see "system" on the CLI, infact we ran the activation scripts, thus the rename.
…on the CLI Required during first-boot of a system. We have images form amd64 and arm64 CPUs which also tend to have different serial interfaces (ttyS vs. ttyAMA). The images which are installed have the correct serial setting for GRUB (ttyS0 or ttyAMA0) and the activation script will probe the Kernel command-line. If a serial interface is defined, we will include it in the VyOS CLI configuration.
Previously, VyOS hardcoded the kernel boot log console to either ttyS0 or tty0, with no post-install CLI method to change it (manual GRUB edits were required). This commit adds a new CLI node: system console device <name> kernel When set, the selected serial console is used as the kernel boot console. When removed, the kernel boot console falls back to tty0.
625e66d to
35db941
Compare
|
CI integration ❌ failed! Details
|
Change summary
Required during first-boot of a system. We have images form amd64 and arm64 CPUs which also tend to have different serial interfaces (
ttySvs.ttyAMA).The images which are installed have the correct serial setting for GRUB (
ttyS0orttyAMA0) and the activation script will probe the Kernel command-line. If a serial interface is defined, we will include it in the VyOS CLI configuration.Types of changes
Related Task(s)
Related PR(s)
How to test / Smoketest result
TBD
Checklist: